Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stack overflow fixes #7151

Merged
merged 28 commits into from
Jul 10, 2019
Merged

Conversation

TIHan
Copy link
Contributor

@TIHan TIHan commented Jul 6, 2019

This is to solve this -> #7070
It adds some stress tests by trying to compile F# code that is very large.

Most of the reported stack overflows at this point are occurring at compile time when we generate IL. Looking at the Optimizer, it recursively goes through linear expression trees in a specific way that would create tail calls. In IlxGen, that wasn't happening for Expr.Sequential/Expr.Let. With this change, it should handle it. We also delay the generation of methods when evaluating expressions; this also prevents stack overflows.

src/fsharp/IlxGen.fs Outdated Show resolved Hide resolved
@TIHan
Copy link
Contributor Author

TIHan commented Jul 6, 2019

Some tests are failing; there are IL gen issues happening. Will need to look at this more.

src/fsharp/IlxGen.fs Outdated Show resolved Hide resolved
src/fsharp/IlxGen.fs Outdated Show resolved Hide resolved
@TIHan TIHan closed this Jul 9, 2019
@TIHan TIHan reopened this Jul 9, 2019
@TIHan TIHan closed this Jul 9, 2019
@TIHan TIHan reopened this Jul 9, 2019
@TIHan
Copy link
Contributor Author

TIHan commented Jul 9, 2019

Getting close, but having some issues with when to evaluate delayed gen methods.

@TIHan
Copy link
Contributor Author

TIHan commented Jul 10, 2019

Alright! We are passing now :)

Copy link
Contributor

@dsyme dsyme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good, just one change requested

src/fsharp/IlxGen.fs Outdated Show resolved Hide resolved
@TIHan TIHan merged commit b3f4e98 into dotnet:master Jul 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants